[No QA][HR Import] Add MergeHRApprovalModePage #91202
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@grgia @cristipaval can I have a polyglot parrot triggered here please ? |
melvin-bot
Bot
requested review from
bernhardoj and
flaviadefaria
and removed request for
a team
|
@bernhardoj Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <OfflineWithFeedback | ||
| pendingAction={card.config?.pendingFields?.approvalMode} | ||
| errors={card.config?.errorFields?.approvalMode} | ||
| onClose={() => clearMergeHRErrorField(policy?.id)} |
There was a problem hiding this comment.
❌ CONSISTENCY-6 (docs)
The clearMergeHRErrorField function requires both policyID and provider parameters, but only policy?.id is passed here. Since provider will be undefined, the function will early-return without clearing the error, making the onClose callback a silent no-op.
Pass the provider name as the second argument. The card object likely has a provider identifier available:
onClose={() => clearMergeHRErrorField(policy?.id, card.providerName)}Reviewed at: d0b9302 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0b930247a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <OfflineWithFeedback | ||
| pendingAction={card.config?.pendingFields?.approvalMode} | ||
| errors={card.config?.errorFields?.approvalMode} | ||
| onClose={() => clearMergeHRErrorField(policy?.id)} |
There was a problem hiding this comment.
onClose calls clearMergeHRErrorField(policy?.id) without a provider, but clearMergeHRErrorField now returns early unless both policyID and provider are present. In the approval-mode error state, dismissing the OfflineWithFeedback error will therefore no-op and the error remains stuck in Onyx/UI. This affects any HR approval mode update failure where the user tries to clear the error.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/es.ts b/src/languages/es.ts
index f57ef47b..80459018 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -6268,7 +6268,7 @@ ${amount} para ${merchant} - ${date}`,
syncError: (providerName: string) => `No se puede conectar con ${providerName}`,
connectionDescription: (providerName: string) => `Conecta ${providerName} para mantener sincronizadas las aprobaciones de empleados con tu espacio de trabajo.`,
approvalMode: 'Modo de aprobación',
- providerApprovalMode: (providerName: string) => `Modo de aprobación de ${providerName}`,
+ providerApprovalMode: (providerName: string) => `modo de aprobación de ${providerName}`,
finalApprover: 'Aprobador final',
notSet: 'No configurado',
approvalModeDescription: (providerName: string) => `Los miembros y gerentes están configurados para sincronizarse con ${providerName}.`,
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 26ed6655..e4104cff 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -7176,7 +7176,7 @@ Ajoutez davantage de règles de dépenses pour protéger la trésorerie de l’e
syncError: (providerName: string) => `Impossible de se connecter à ${providerName}`,
connectionDescription: (providerName: string) => `Connectez ${providerName} pour synchroniser les approbations des employés avec votre espace de travail.`,
approvalMode: "Mode d'approbation",
- providerApprovalMode: (providerName: string) => `Mode d'approbation ${providerName}`,
+ providerApprovalMode: (providerName: string) => `Mode d’approbation ${providerName}`,
finalApprover: 'Approbateur final',
notSet: 'Non défini',
approvalModeDescription: (providerName: string) => `Les membres et les responsables sont configurés pour se synchroniser avec ${providerName}.`,
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 26204ac3..6d100d4d 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -7059,7 +7059,7 @@ ${reportName}
syncError: (providerName: string) => `${providerName}に接続できません`,
connectionDescription: (providerName: string) => `${providerName}を接続して、従業員の承認をワークスペースと同期させましょう。`,
approvalMode: '承認モード',
- providerApprovalMode: (providerName: string) => `${providerName}の承認モード`,
+ providerApprovalMode: (providerName: string) => `${providerName} 承認モード`,
finalApprover: '最終承認者',
notSet: '未設定',
approvalModeDescription: (providerName: string) => `メンバーとマネージャーは ${providerName} と同期するように設定されています。`,
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 58689355..3d6d04a0 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -6932,7 +6932,7 @@ ${reportName}
syncError: (providerName: string) => `无法连接到 ${providerName}`,
connectionDescription: (providerName: string) => `连接 ${providerName}，以在您的工作区中同步员工审批。`,
approvalMode: '审批模式',
- providerApprovalMode: (providerName: string) => `${providerName}审批模式`,
+ providerApprovalMode: (providerName: string) => `${providerName} 审批模式`,
finalApprover: '最终审批人',
notSet: '未设置',
approvalModeDescription: (providerName: string) => `成员和管理员已设置为与 ${providerName} 同步。`,
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
@jmusial will need to merge main |
This comment has been minimized.
This comment has been minimized.
| export {syncMergeHR, updateMergeHRApprovalMode, updateMergeHRFinalApprover}; | ||
| type HRProviderName = TupleToUnion<typeof CONST.POLICY.CONNECTIONS.HR_CONNECTION_NAMES>; | ||
|
|
||
| function clearMergeHRErrorField(policyID?: string, provider?: HRProviderName) { |
There was a problem hiding this comment.
I think instead of making the params optional, we should update the type to be undefined-able.
| function clearMergeHRErrorField(policyID?: string, provider?: HRProviderName) { | |
| function clearMergeHRErrorField(policyID: string | undefined, provider: HRProviderName | undefined) { |
This will prevent unintended bug like the on caught by AI here
| connections: { | ||
| [provider]: { | ||
| config: { | ||
| errorFields: null, |
There was a problem hiding this comment.
This clears the whole errorFields. If we have both approvalMode and finalApprover errorFields, clearing one of them will clear both.
|
Is there a test account I can use to connect? |
yes, I sent it to you on Slack just now |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.mp4Android: mWeb Chromeandroid.mweb.mp4iOS: HybridAppios.mp4iOS: mWeb Safariios.mweb.mp4MacOS: Chrome / Safariweb.mp4 |
|
The confirmation modal is missing the confirmation text on native. 
|
|
I can't seem to connect to the HR provider on iOS 🤔 ioss.mp4 |
Fixed |
yup, we're aware of the issue, it will be solved here |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.79-1 🚀
Bundle Size Analysis (Sentry): |
5 participants
Explanation of Change
This PR adds
MergeHRApprovalModePageto the Merge dev flows. It also unifies Gustos and MergeApproval Mode pages.
Fixed Issues
$ #90602
PROPOSAL:
Tests
Pre requisites:
mergeHRConnectionsflag onSteps:
Offline tests
Same as test steps.
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-05-21.at.11.44.35.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-21.at.11.43.00.mov